Conversation
1) Use REST instead of SOAP 2) Refactoring code
src/Service/FedExRequest.cs
Outdated
| { | ||
| public static string SendRequest(string baseAddress, CommandConfiguration configuration) | ||
| { | ||
| if (configuration.CommandType is not ApiCommand.CreateAccessToken) |
There was a problem hiding this comment.
This check seems rather odd - we can never get to SendRequest if it's not CreateAccessToken?
There was a problem hiding this comment.
To SendRequest when we have Access token, we need to use the method with TokenData? tokenData argument.
I can just remove this one SendRequest(string baseAddress, CommandConfiguration configuration) if it looks better.
There was a problem hiding this comment.
Yeah I think it's a little confusing, if it's only when we have an access token this SendRequest is called, then either we should rename it to something that says so, or remove it :)
There was a problem hiding this comment.
Removed.
src/Service/FedExRequest.cs
Outdated
|
|
||
| public static string SendRequest(string baseAddress, CommandConfiguration configuration, TokenData? tokenData) | ||
| { | ||
| using (var messageHandler = GetMessageHandler()) |
There was a problem hiding this comment.
Please try to avoid all this excess indentation
There was a problem hiding this comment.
Fixed.
src/Service/FedExRequest.cs
Outdated
| ApiCommand.CreateAccessToken => client.PostAsync(apiCommand, GetFormUrlEncodedContent(configuration)), | ||
| ApiCommand.GetRateAndTransitTimes or | ||
| ApiCommand.ValidateAddress => client.PostAsync(apiCommand, GetStringContent(configuration)), | ||
| _ => throw new NotSupportedException($"Unknown operation was used. The operation code: {configuration.CommandType}.") |
There was a problem hiding this comment.
Throw NotImplementedException
src/Service/FedExRequest.cs
Outdated
| } | ||
| catch (HttpRequestException requestException) | ||
| { | ||
| throw new Exception($"An error occurred during FedEx request. Error code: {requestException.StatusCode}"); |
There was a problem hiding this comment.
Logged before throwing it.
Exception is catched by try-catch in the caller, so exception should be thrown outside of the method.
Like here:
Or here:
But I added logging of this error, so we can read it in the logs.
src/Service/FedExRequest.cs
Outdated
| ApiCommand.CreateAccessToken => GetCommandLink("oauth/token"), | ||
| ApiCommand.GetRateAndTransitTimes => GetCommandLink("rate/v1/rates/quotes"), | ||
| ApiCommand.ValidateAddress => GetCommandLink($"address/v1/addresses/resolve"), | ||
| _ => throw new NotSupportedException($"The api command is not supported. Command: {configuration.CommandType}") |
There was a problem hiding this comment.
Throw NotImplementedException
2 participants
Removed the field "ServiceUrl" - we don't need it anymore, since use the REST API.
Added "Test Mode" checkbox to use sandbox environment for test. Uncheck for production.
Renamed some fields according to actual names of parameters in FedEx admin panel.
Refactored code to use our common Client-Server communication layer which we use in other payment/shipping providers.
Added logging of each data we send to server and the data which we get from server.
Some other related changes also were done, but they are mainly related with parameters processing...
Added logo.
Used Net 8.0 Framework.
Activated Nullable as per task description.
The settings of provider now looks like this:
The original task: User Story 23886. Rewrite FedEx provider to use REST